Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update channel_update_tutorial.rst to follow the latest fabric-samples #4770

Merged
merged 1 commit into from
Aug 20, 2024

Conversation

satota2
Copy link
Contributor

@satota2 satota2 commented Mar 28, 2024

Type of change

  • Bug fix
  • Documentation update

Description

Minor updates on channel_update_tutorial.rst to follow the latest fabric-samples.

Additional details

Related issues

@satota2 satota2 requested review from a team as code owners March 28, 2024 03:01
Copy link
Member

@yeasy yeasy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simply update the version number from 1 to 1.0.1

@satota2
Copy link
Contributor Author

satota2 commented Aug 19, 2024

@hyperledger/fabric-core-maintainers
It's a very minor fix, but it's important to keep the documentation up to date.
I would appreciate it if someone could take the time to review and merge it.

@@ -579,7 +579,7 @@ The first step is to package the Basic chaincode:

.. code:: bash

peer lifecycle chaincode package basic.tar.gz --path ../asset-transfer-basic/chaincode-go/ --lang golang --label basic_1
peer lifecycle chaincode package basic.tar.gz --path ../asset-transfer-basic/chaincode-go/ --lang golang --label basic_1.0.1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for not reviewing sooner, I think I did not understand the intent of why use v1.0.1 instead of v1. Could you explain? Does test-network default to v1.0.1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@denyeart

Apologies for not reviewing sooner, I think I did not understand the intent of why use v1.0.1 instead of v1. Could you explain? Does test-network default to v1.0.1?

No problem at all, please don’t worry about it. Thank you for your review.

The latest test-network defaults to using version 1.0.1, as indicated in this code. My intention was to align with this update.

While addressing your comment, I noticed a discrepancy regarding the default chaincode version in the latest test-network (e.g., 1.0.1, 1.0.0, 1.0), including an inconsistency in the command message description.

Would it be more appropriate to resolve this discrepancy by unifying the default value to '1' in the fabric-samples repository, rather than addressing it through this PR? I would appreciate your thoughts on the best approach.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the value of CC_VERSION="1.0.1" was a mistake. Prior to this commit it was "1.0"

And in the lifecycle ReadTheDoc tutorials it is "1.0".

I think "1.0" was chosen for samples to differentiate it from the sequence number which is an integer like "1".

I'd suggest to align the samples default and the tutorials to be "1.0".

Thank you for looking into these details!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@denyeart
Thank you for your response.

I agree with your suggestion. To align with the tutorials, I will create and submit a patch to fix the version to “1.0” in the fabric-samples repository. Once that's done, I will update this PR accordingly.

Thank you again for your guidance!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@denyeart
I updated the PR to align to "1.0".
Also, I submitted the following PR in fabric-samples.
hyperledger/fabric-samples#1245

Please confirm them.

Signed-off-by: Tatsuya Sato <tatsuya.sato.so@hitachi.com>
Copy link
Contributor

@denyeart denyeart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@denyeart denyeart merged commit 48baa59 into hyperledger:main Aug 20, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants